fix: Don't auto-close flyout for new vars#9176
fix: Don't auto-close flyout for new vars#9176BenHenning wants to merge 1 commit intoRaspberryPiFoundation:mainfrom
Conversation
This isn't a complete solution, but is a decent start toward one. It also builds resilience into Flyout itself for recreation cases.
|
@BenHenning this is a quite old draft, do you plan on updating this to be able to get it merged before the user testing deadline? |
For posterity we're trying to figure out whether this is an actual blocker for January testing, though it would be nice to find a solution. To that end I have investigated this more and found that it's very much still an issue. I was wondering a bit whether #9245 might affect this because it changed a bit with auto hiding behavior, but the alert dialog situation is still very much broken. Fundamentally what's happening here is that the browser seems to send a
Both approaches are non-trivial and will require a lot of multi-browser and multi-screenreader testing to feel good about them. |
|
Actually based on the above message I am going to close this and update #9122 accordingly. |
The basics
The details
Resolves
Fixes #9106
Fixes #9122
Proposed Changes
This ensures
Flyoutdoesn't close when it shouldn't (i.e. when creating or renaming variables).Reason for Changes
The main issue is that the window prompt takes focus away from Blockly entirely, so ephemeral focus is now being used for variable creation and renaming to restore focus back to the original node.
However, that's not a complete solution since creating a variable causes the flyout to fully be laid out again. As a result, there's no trivial way to track and restore focus back to the element that had held focus previously (or to use the ephemerally-restored focus to even realize that focus should be restored). To make this work reasonably well, when the
Flyoutis re-laid out it will now check if it had focus before and, if it did, will focus the first new element it creates after being relaid out. It's not a perfect solution, but it seems like a reasonable medium or long-term solution.Test Coverage
TODO: Tests need to be added yet.
Documentation
No new documentation changes should be needed.
Additional Information
Note that this PR is not production ready yet as it completely disables auto-flyout closing. The main issue here is that there's a moment either due to the prompt or due to elements with focus being deleted that eventually lead to the flyout's workspace being blurred and attempts to restore focus after that completely fail since it's been closed and the nodes no longer exist. It's unclear at the moment how to solve this problem.